Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Issue #3511 fixed #3513

Closed
wants to merge 13 commits into from
Closed

Issue #3511 fixed #3513

wants to merge 13 commits into from

Conversation

jainsneha23
Copy link
Contributor

Call on-toggle function of dropdown in resolve function of $animate

@chrisirhc
Copy link
Contributor

It looks like with this change, you need to do $browser.defer.flush() in the dropdown.spec for the failing tests because the toggleHandler is now deferred.

@chrisirhc chrisirhc added this to the Backlog milestone Apr 13, 2015
@rvanbaalen
Copy link
Contributor

Since this related issue, #3511, is labeled Angular 1.3 I've added the Angular 1.3 label here as well.
@chrisirhc isn't this part of Angular 1.3 support?

@chrisirhc
Copy link
Contributor

@rvanbaalen not exactly, it's actually an ngAnimate issue but nonetheless we should try to have this fixed for next release.

@chrisirhc chrisirhc modified the milestones: 0.13.x, Backlog Apr 14, 2015
@chrisirhc
Copy link
Contributor

@snehatulsi , could you please add $browser.defer.flush() in the test code so that the tests pass?

@jainsneha23
Copy link
Contributor Author

jainsneha23 commented Apr 16, 2015 via email

@chrisirhc
Copy link
Contributor

No problem, the tests are located at: https://github.com/angular-ui/bootstrap/blob/master/src/dropdown/test/dropdown.spec.js

You can see the failures when you run grunt on your command line once you've prepared your environment.
The test failures will indicate the lines where it's failing at like:

screen shot 2015-04-16 at 11 04 47 am

@jainsneha23
Copy link
Contributor Author

@chrisirhc I tried adding the code. I got the error $browser not defined. I tried with modifications, but failed. I don't know the test specs, I have never done testing this way. Please mention where to inject $browser dependency, and where to insert code for $browser.defer.flush()


beforeEach(module('ui.bootstrap.dropdown'));

beforeEach(inject(function(_$compile_, _$rootScope_, _$document_) {
beforeEach(inject(function(_$compile_, _$rootScope_, _$document_, $browser) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use _$browser_ here

@chrisirhc
Copy link
Contributor

Added a comment. You need to add the flush calls just before the expect calls on the lines that they're failing.

The flush call ensures that all promises have been resolved, since you moved the toggleHandler to be called in a then. It's required for the tests to get to the correct state before calling expect.

@jainsneha23
Copy link
Contributor Author

@chrisirhc The test passed

@karianna
Copy link
Contributor

@snehatulsi Can you update this PR, it can no longer be merged against master

@wesleycho
Copy link
Contributor

Can you squash your commits & rebase off of master?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants